O3-2448 - Visit queue number customization #44

mseaton · 2023-10-24T00:38:37Z

The goal of this PR is to address the issues raised in https://issues.openmrs.org/browse/O3-2448 and https://issues.openmrs.org/browse/O3-2465. This PR does the following:

Adds patientQueueNumber / patient_queue_number to QueueEntry / queue_entry, in order to keep this information directly in the queue model and remove the need for a custom visit attribute.
Removes the bespoke code for generating queue numbers from within the core DAO and into an implementation behind an interface. This enables this functionality to be off by default, added easily if needed, and customized however implementations want.
Removes the services and rest endpoints for generating and saving queue numbers as a standalone operation in favor of one that embeds this functionality in the visit queue number saving process. This enables transactional behavior around generating and setting visit queue numbers, visit attributes, visit details, and queue entry details at the same time, rather than requiring consuming code orchestrate these steps together non-transactionally.

See specific comments throughout the PR.

I have this as a Draft PR initially to show where I've been heading and looking for feedback / suggestions. This will obviously be disruptive and some work to get it working as expected in existing implementations that rely on current behavior. It will also need to be paired with changes to the O3 frontend to remove the calls to generate visit numbers, etc.

@corneliouzbett / @CynthiaKamau / @ibacher interested in thoughts / feedback. @mogoodrich FYI

mseaton · 2023-10-24T02:51:26Z

api/src/main/java/org/openmrs/module/queue/api/dao/impl/QueueEntryDaoImpl.java

@@ -74,42 +64,6 @@ public Long getQueueEntriesCountByConceptStatus(@NotNull String conceptStatus, C
 		return (Long) criteria.uniqueResult();
 	}

-	@Override


The logic here (combined with the logic in the service below) has essentially been moved to the VisitAttributeQueueNumberProcessor. Implementations that want to retain this behavior can enable that processor by setting it on the service from a downstream module. That said, if we don't feel this is something we need/want to keep in the main codebase, I'd be supportive of moving the entire VisitAttributeQueueNumberProcessor over to a Palladium module, and not having to support that within the module.

Note, the code relocated to the processor is not exactly the same. It does not hit the DAO, but rather just iterates over the entries in the queue to achieve the same result. This may need to be optimized for speed, but didn't want to do that pre-emptively if this ultimately gets moved out of the module.

It was never really clear to me what purpose the location had here, given that queue has a location on it. If there needs to be different location than that of the queue or the visit, this will need to be looked at. That said, there are some potential issues in this method as documented in O3-2448, so it likely needs a review overally

mseaton · 2023-10-24T02:54:59Z

api/src/main/java/org/openmrs/module/queue/api/impl/VisitQueueEntryServiceImpl.java

-			        .createQueueEntry(visitQueueEntry.getQueueEntry());
-			visitQueueEntry.setQueueEntry(newlyCreatedQueueEntry);
-			return this.dao.createOrUpdate(visitQueueEntry);
+	public synchronized VisitQueueEntry createVisitQueueEntry(@NotNull VisitQueueEntry visitQueueEntry) {


Made this method synchronized, as there tend to be situations where the queue number to generate is based on the queue entries and/or existing queue numbers assigned that exist in the database, and may need to protect against several threads generating the same queue number at the same time in different threads. See some of the comments in O3-2448

mseaton · 2023-10-24T02:57:38Z

api/src/main/java/org/openmrs/module/queue/api/impl/VisitQueueEntryServiceImpl.java

+		visitQueueEntry = dao.createOrUpdate(visitQueueEntry);
+
+		if (visitQueueEntryProcessor != null) {
+			visitQueueEntryProcessor.beforeSaveVisitQueueEntry(visitQueueEntry);


This is the main area that addresses the issue and allows customization. If a visitQueueEntryProcessor is set on the service, then it will be called to generate and/or populate any properties of the visitQueueEntry prior to final saving and completion of the transaction. An initial call to save happens before this is invoked in case this want to use generated data from the first save (for example, the primary key id).

mseaton · 2023-10-24T02:59:13Z

api/src/main/java/org/openmrs/module/queue/processor/BasicPatientQueueNumberGenerator.java

+ */
+@Slf4j
+@Component
+public class BasicPatientQueueNumberGenerator implements VisitQueueEntryProcessor {


I originally had a different implementation here that was a bit more complex, but in the end I decided to make this as basic as possible. This may mean this is never appropriate for use in any real implementation, but it does serve to provide a default implementation and an example of how one would create a custom generator.

mseaton · 2023-10-24T02:59:41Z

api/src/main/java/org/openmrs/module/queue/processor/VisitAttributeQueueNumberGenerator.java

+
+/**
+ * Implementation of a VisitQueueEntryProcessor which aims to retain legacy functionality using
+ * visit attributes


This is where I've attempted to recreate the logic from the original DAO methods that were removed.

mseaton · 2023-10-24T03:00:49Z

api/src/main/java/org/openmrs/module/queue/processor/VisitAttributeQueueNumberGenerator.java

+		String queueNumber = prefix + "-" + paddedString;
+
+		// Associate this queue number directly
+		queueEntry.setPatientQueueNumber(queueNumber);


This was not in the original implementation, but I see no harm in it, and I think for a generic front-end implementation we'll need this - the data for queue number in a known, fixed attribute of the queue entry - even if is also duplicated elsewhere (eg. a custom visit attribute).

mseaton · 2023-10-24T03:01:41Z

api/src/main/java/org/openmrs/module/queue/processor/VisitAttributeQueueNumberGenerator.java

+		VisitAttribute visitAttribute = new VisitAttribute();
+		visitAttribute.setAttributeType(visitAttributeType);
+		visitAttribute.setValueReferenceInternal(queueNumber);
+		visitQueueEntry.getVisit().addAttribute(visitAttribute);


It's possible / likely that with the new column, this is not longer behavior that is desired. But I'll leave that to others to discuss and decide.

mseaton · 2023-10-24T03:02:20Z

api/src/test/java/org/openmrs/module/queue/api/dao/QueueEntryDaoTest.java

@@ -244,16 +241,4 @@ public void shouldZeroCountQueueEntriesByBadStatus() {
 		assertThat(queueEntriesCountByStatusCount, notNullValue());
 		assertThat(queueEntriesCountByStatusCount, is(0L));
 	}
-


This test was moved / reimplemented below.

mseaton · 2023-10-24T03:03:15Z

...test/resources/org/openmrs/module/queue/api/dao/QueueEntryDaoTest_conceptsInitialDataset.xml

@@ -13,8 +13,8 @@
    <concept concept_id="1000" retired="false" is_set="true" creator="1" date_created="2022-02-02 14:31:00.0" uuid="897eba27-2b38-43e8-91a9-4dfe3956a35t"/>
    <concept concept_id="1001" retired="false" is_set="false" creator="1" date_created="2022-02-02 14:40:00.0" uuid="90b910bd-298c-4ecf-a632-661ae2f446op"/>
    <concept concept_id="1002" retired="false" is_set="false" creator="1" date_created="2022-02-02 14:40:00.0" uuid="91b910bd-298c-4ecf-a632-661ae2f909ut"/>


The below are bugs that I found - uuids and concept name ids that were re-used and not unique

mseaton · 2023-10-25T21:07:39Z

Closing this PR for now as I'm rethinking this approach and working on other issues.

mseaton added 5 commits October 23, 2023 20:38

O3-2448 - Visit queue number customization

b268578

O3-2448 - Visit queue number customization

e796bdb

O3-2448 - Visit queue number customization

6707946

O3-2448 - Visit queue number customization

0aa2b3a

O3-2448 - Visit queue number customization

4bd7ec3

mseaton commented Oct 24, 2023

View reviewed changes

mseaton requested review from corneliouzbett, CynthiaKamau and ibacher October 24, 2023 03:03

mseaton closed this Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

O3-2448 - Visit queue number customization #44

O3-2448 - Visit queue number customization #44

mseaton commented Oct 24, 2023 •

edited

Loading

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton Oct 24, 2023

mseaton commented Oct 25, 2023

O3-2448 - Visit queue number customization #44

O3-2448 - Visit queue number customization #44

Conversation

mseaton commented Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mseaton commented Oct 25, 2023

mseaton commented Oct 24, 2023 •

edited

Loading